Skip to content

Conversation

@yulesxoxo
Copy link

@yulesxoxo yulesxoxo commented Oct 5, 2025

Description

Fairly minimal repo to reproduce the problem: https://github.com/yulesxoxo/wails-dnd-dropzone-repro with videos and all that.

(Copied from Issue)
File drops were not registering correctly when display scaling was set to values other than 100%.

Setting the entire HTML content as a dropzone would work at 100% but at 200% scaling, dropping files would only register in the top left quadrant.

I believe the problem to be that the platform provides file drop coordinates in physical/device pixels, but document.elementFromPoint() expects CSS pixels (logical coordinates). Without conversion, at 150% scaling (devicePixelRatio = 1.5), a drop at physical coordinates (1200, 900) would be interpreted as CSS (1200, 900), causing elementFromPoint to look far outside the actual drop location.

Edit:
After some testing it looks like this issue is not present on macOS but only on Windows. Can't test on Linux but I made changes so it only applies to Windows

Fixes #4631

Type of change

Please select the option that is relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration using wails doctor.

  • Windows
  • macOS
  • Linux

If you checked Linux, please specify the distro and version.

Test Configuration

Wails (v3.0.0-alpha.33)  Wails Doctor 
                                                                                                                                                                                        
# System 

┌────────────────────────────────────────────────────────────────────────────────────────────┐
| Name              | Windows 10 Pro                                                         |
| Version           | 2009 (Build: 26100)                                                    |
| ID                | 24H2                                                                   |
| Branding          | Windows 11 Pro                                                         |
| Platform          | windows                                                                |
| Architecture      | amd64                                                                  |
| Go WebView2Loader | true                                                                   |
| WebView2 Version  | 140.0.3485.94                                                          |
| CPU               | 13th Gen Intel(R) Core(TM) i7-13700K                                   |
| GPU 1             | NVIDIA GeForce RTX 4090 (NVIDIA) - Driver: 32.0.15.8129                |
| GPU 2             | Intel(R) UHD Graphics 770 (Intel Corporation) - Driver: 32.0.101.6987  |
| Memory            | 64GB                                                                   |
└────────────────────────────────────────────────────────────────────────────────────────────┘

# Build Environment 

┌────────────────────────────────┐
| Wails CLI    | v3.0.0-alpha.33 |
| Go Version   | go1.24.0        |
| -buildmode   | exe             |
| -compiler    | gc              |
| CGO_CFLAGS   |                 |
| CGO_CPPFLAGS |                 |
| CGO_CXXFLAGS |                 |
| CGO_ENABLED  | 1               |
| CGO_LDFLAGS  |                 |
| GOAMD64      | v1              |
| GOARCH       | amd64           |
| GOOS         | windows         |
└────────────────────────────────┘

# Dependencies 

┌────────────────────────────────────────────┐
| MakeAppx.exe (Windows SDK) | Not Installed |
| MSIX Packaging Tool        | Not Installed |
| SignTool.exe (Windows SDK) | Not Installed |
| npm                        | 10.9.3        |
| NSIS                       | Not Installed |
|                                            |
└───────── * - Optional Dependency ──────────┘

# Checking for issues 

 SUCCESS  No issues found

# Diagnosis 

 SUCCESS  Your system is ready for Wails development!

Checklist:

  • I have updated website/src/pages/changelog.mdx with details of this PR
  • My code follows the general coding style of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

Summary by CodeRabbit

  • Bug Fixes

    • Fixes file drag-and-drop detection on Windows with display scaling so drops target the correct element across high‑DPI and zoomed displays; preserves behavior at standard DPI and other platforms.
  • Documentation

    • Clarified that file-drop handler coordinates are in logical (CSS) pixels relative to the webview.
  • Chores

    • Added a changelog entry describing the fix.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 5, 2025

Walkthrough

Added DPI-aware coordinate conversion on Windows to translate physical drop coordinates to logical (CSS) pixels before hit-testing; updated JSDoc for drop coordinates and inserted an unreleased changelog entry. No public API signature changes.

Changes

Cohort / File(s) Summary
Windows webview coordinate conversion
v3/pkg/application/webview_window_windows.go
Compute webview physical coordinates, retrieve DPI (with 0-dpi fallback), apply scaleFactor (dpi/96), convert to logical/CSS pixels using rounding, return logical coordinates; added math import and expanded debug logging.
Runtime docs (parameter docs only)
v3/internal/runtime/desktop/@wailsio/runtime/src/window.ts
Updated JSDoc for HandlePlatformFileDrop parameters x and y to state they are logical (CSS) pixels relative to the webview; no runtime logic changes.
Changelog
v3/UNRELEASED_CHANGELOG.md
Added Fixed entry: "Fixed DnD dropzone detection failing at non-100% display scaling on Windows (#4632)".

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor User as User
  participant OS as Platform (File Drop)
  participant Win as Windows runtime
  participant DOM as WebView / DOM

  User->>OS: Drop files at (x,y) [physical/device pixels]
  OS->>Win: HandlePlatformFileDrop(x,y, files)
  Note over Win: GetDpiForWindow -> dpi\nscaleFactor = dpi / 96\ncompute webviewPhysicalX/Y\nwebviewLogicalX = round(webviewPhysicalX / scaleFactor)
  Win->>DOM: elementFromPoint(webviewLogicalX, webviewLogicalY)
  DOM-->>Win: targetElement
  alt target has `data-wails-dropzone`
    Win->>Win: Emit drop payload to listeners (includes coords)
  else not a dropzone
    Win->>Win: Log/ignore drop
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Areas to focus on:

  • DPI retrieval and fallback handling (GetDpiForWindow, dpi == 0).
  • Correct mapping between window/client coordinates and webview offsets.
  • Rounding behavior and potential off-by-one at boundaries due to rounding.

Suggested labels

Bug, size:M

Suggested reviewers

  • leaanthony

Poem

A hop, a drop, a pinpoint beat —
I scale the pixels, make coords meet.
From device to CSS I bound,
So every file finds common ground.
🐇📦

Pre-merge checks and finishing touches

✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the primary fix: addressing DnD dropzone detection failure at non-100% display scaling on Windows.
Description check ✅ Passed The PR description follows the template with all major sections completed: summary, type of change, testing details, test configuration, and checklist items.
Linked Issues check ✅ Passed The code changes directly address the requirements from issue #4631: converting physical/device pixels to CSS pixels for file drop coordinates on Windows to fix dropzone detection at non-100% display scaling.
Out of Scope Changes check ✅ Passed All changes are directly scoped to fixing the dropzone scaling issue on Windows: coordinate conversion in the Windows webview handler, documentation update, and changelog entry.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5f835c0 and c2832a9.

📒 Files selected for processing (1)
  • v3/pkg/application/webview_window_windows.go (2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-10-17T23:16:11.570Z
Learnt from: Sammy-T
Repo: wailsapp/wails PR: 4570
File: v2/internal/frontend/desktop/linux/window_webkit6.go:97-108
Timestamp: 2025-10-17T23:16:11.570Z
Learning: For webkit_6/GTK4 builds in v2/internal/frontend/desktop/linux/window_webkit6.go, GTK widget creation should not be wrapped in invokeOnMainThread. The activation mechanism (activateWg + onActivate export) already handles thread safety, and additional wrapping would cause issues.

Applied to files:

  • v3/pkg/application/webview_window_windows.go
📚 Learning: 2024-09-20T23:34:29.841Z
Learnt from: nixpare
Repo: wailsapp/wails PR: 3763
File: v3/examples/keybindings/main.go:16-17
Timestamp: 2024-09-20T23:34:29.841Z
Learning: In the codebase, `application.Options.KeyBindings` uses the `application.Window` type, whereas `application.WebviewWindowOptions.KeyBindings` uses `*application.WebviewWindow`. This is intentional and acceptable.

Applied to files:

  • v3/pkg/application/webview_window_windows.go
🧬 Code graph analysis (1)
v3/pkg/application/webview_window_windows.go (1)
v3/pkg/w32/user32.go (1)
  • GetDpiForWindow (383-386)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: Run Go Tests v3 (windows-latest, 1.24)
  • GitHub Check: Run Go Tests v3 (ubuntu-latest, 1.24)

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@yulesxoxo yulesxoxo changed the title fix: convert physical pixels to CSS pixels for file drop coordinates fix: convert physical pixels to CSS pixels to resolve dropzone when display scaling is active Oct 5, 2025
@yulesxoxo yulesxoxo changed the title fix: convert physical pixels to CSS pixels to resolve dropzone when display scaling is active fix: convert physical pixels to CSS pixels to resolve dropzone when display scaling is used Oct 5, 2025
@yulesxoxo yulesxoxo changed the title fix: convert physical pixels to CSS pixels to resolve dropzone when display scaling is used fix: DnD dropzone detection failing at non-100% display scaling Oct 5, 2025
@github-actions github-actions bot added the Documentation Improvements or additions to documentation label Oct 5, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
v3/internal/runtime/desktop/@wailsio/runtime/src/window.ts (1)

555-555: Consider clarifying coordinate system in console logs.

The console logs reference coordinates as ${x},${y}, but the element was actually located using the converted CSS coordinates (cssX, cssY). For debugging clarity, consider logging both physical and CSS coordinates, or at least note which coordinate system is being logged.

Example enhancement:

-console.log(`Wails Runtime: Drop on element (or no element) at ${x},${y} which is not a designated dropzone. Ignoring. Element:`, element);
+console.log(`Wails Runtime: Drop at physical (${x},${y}) / CSS (${cssX},${cssY}) on element which is not a designated dropzone. Ignoring. Element:`, element);
-console.log(`Wails Runtime: Drop on designated dropzone. Element at (${x}, ${y}):`, element, 'Effective dropzone:', dropzoneTarget);
+console.log(`Wails Runtime: Drop on designated dropzone. Element at physical (${x}, ${y}) / CSS (${cssX}, ${cssY}):`, element, 'Effective dropzone:', dropzoneTarget);

Also applies to: 560-560

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f1037c8 and 5af39d3.

📒 Files selected for processing (1)
  • v3/internal/runtime/desktop/@wailsio/runtime/src/window.ts (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
v3/internal/runtime/desktop/@wailsio/runtime/src/window.ts (1)
v3/internal/assetserver/bundledassets/runtime.debug.js (3)
  • element (1025-1025)
  • element (1138-1138)
  • element (1337-1337)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: Run Go Tests v3 (windows-latest, 1.24)
  • GitHub Check: Run Go Tests v3 (macos-latest, 1.24)
  • GitHub Check: Run Go Tests v3 (ubuntu-latest, 1.24)
  • GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (2)
v3/internal/runtime/desktop/@wailsio/runtime/src/window.ts (2)

546-549: LGTM! Coordinate conversion correctly fixes the scaling bug.

The fix properly converts physical pixel coordinates (from the platform) to CSS pixel coordinates (expected by document.elementFromPoint) by dividing by window.devicePixelRatio. The fallback to 1 handles the edge case where the property might be undefined. This directly addresses the root cause described in issue #4631.


573-574: Inspect JS drop coordinate source in window.ts

Please run:

#!/bin/bash
# Locate window.ts
fd "window.ts" -I

# Show lines around 570
sed -n '560,590p' v3/internal/runtime/desktop/@wailsio/runtime/src/window.ts

Verify whether x and y originate from CSS (e.g., clientX/clientY) or are already multiplied by window.devicePixelRatio.

Comment on lines 546 to 549
const scale = window.devicePixelRatio || 1;
const cssX = x / scale;
const cssY = y / scale;
const element = document.elementFromPoint(cssX, cssY);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Ensure bundled runtime files are regenerated.

The relevant code snippets show that v3/internal/assetserver/bundledassets/runtime.debug.js contains similar document.elementFromPoint(x, y) calls at lines 1024 and 1336. These bundled files should be regenerated from this TypeScript source after merging to ensure the fix is included in the distributed runtime.

Verify the build process regenerates bundled files:


🏁 Script executed:

#!/bin/bash
# Description: Check if bundled runtime files are auto-generated or need manual rebuild

# Look for build/generation scripts
fd -t f -e sh -e js -e go 'build|generate|bundle' -x grep -l 'runtime.*bundle\|bundled.*runtime' {}

# Check if there are build instructions or comments about regenerating bundles
rg -n -C 3 'regenerate|rebuild.*bundle|bundle.*rebuild' --type=md --type=go --type=sh

Length of output: 1220


Add bundling of runtime assets to build process.

The bundled runtime files (e.g. v3/internal/assetserver/bundledassets/runtime.debug.js) must be regenerated after updating window.ts to include this fix. There’s no existing automation or documentation for this—update your build/bundle scripts or document the manual rebuild steps.

🤖 Prompt for AI Agents
In v3/internal/runtime/desktop/@wailsio/runtime/src/window.ts around lines
546-549, you changed the runtime and must regenerate the bundled runtime assets
(e.g., v3/internal/assetserver/bundledassets/runtime.debug.js) so the build
includes the updated window.ts; update the repo build scripts (or add a new
script) to run the runtime bundling step after transpiling the runtime sources
(or document the exact manual commands developers must run) and then regenerate
and commit the bundled runtime files so CI and consumers get the patched
runtime.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
v3/internal/runtime/desktop/@wailsio/runtime/src/window.ts (1)

548-548: Consider platform detection from backend.

The user agent check is simple but potentially fragile. If the backend already knows the platform (which it does, since it's Go code running on the host OS), consider passing a platform identifier to the runtime initialization instead of relying on user agent parsing.

This could be a future enhancement—the current approach is acceptable for now since Wails controls the WebView environment, but a more robust solution would query the platform from the Go backend.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a0ca038 and 686d5df.

📒 Files selected for processing (1)
  • v3/internal/runtime/desktop/@wailsio/runtime/src/window.ts (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
v3/internal/runtime/desktop/@wailsio/runtime/src/window.ts (1)
v3/internal/assetserver/bundledassets/runtime.debug.js (3)
  • element (1025-1025)
  • element (1138-1138)
  • element (1337-1337)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: auto-label
  • GitHub Check: Run Go Tests v3 (windows-latest, 1.24)
  • GitHub Check: Run Go Tests v3 (macos-latest, 1.24)
  • GitHub Check: Run Go Tests v3 (ubuntu-latest, 1.24)
🔇 Additional comments (1)
v3/internal/runtime/desktop/@wailsio/runtime/src/window.ts (1)

577-582: Clarify or normalize coordinate space in file-drop payload
You’re forwarding raw x,y (physical pixels on Windows, CSS pixels elsewhere) even though you hit-test with cssX,cssY. Confirm downstream consumers of DropZoneDetails.X/Y expect this mixed space—or switch the payload to use cssX/cssY for consistency.

Comment on lines 546 to 555
let element: Element | null;

if (navigator.userAgent.includes("Windows")) {
const scale = window.devicePixelRatio || 1;
const cssX = x / scale;
const cssY = y / scale;
element = document.elementFromPoint(cssX, cssY);
} else {
element = document.elementFromPoint(x, y);
}
Copy link
Contributor

@coderabbitai coderabbitai bot Oct 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

🧩 Analysis chain

Verify coordinate system on other platforms.

The Windows-specific coordinate conversion correctly addresses the scaling bug by converting physical pixels to CSS pixels before calling document.elementFromPoint(). The math is sound (cssX = x / scale).

However, verify that macOS and Linux platforms pass CSS pixels (not physical pixels) from their native drag-and-drop implementations. If they also pass physical pixels, this fix should apply universally rather than being Windows-specific.

Run the following script to check if platform-specific coordinate handling exists elsewhere in the codebase:


🏁 Script executed:

#!/bin/bash
# Description: Search for platform-specific coordinate or DPI handling in native code

# Search for devicePixelRatio, DPI, or scaling in Go backend files
rg -n -C 3 -g '*.go' 'devicePixelRatio|dpi|scaling|physical.*pixel|css.*pixel' v3/

# Search for file drop coordinate handling in platform-specific code
rg -n -C 5 -g '*.go' 'FileDrop|DropZone' v3/internal/runtime/desktop/

Length of output: 14925


Implement DPI scaling for non-Windows platforms.

Linux and macOS native layers still pass raw (physical) coordinates (see webview_window_linux.go and webview_window_darwin.go “TODO: proper DPI scaling”). Apply the same devicePixelRatio-based conversion (cssX = x/scale, cssY = y/scale) to file-drop handlers on these platforms.

🤖 Prompt for AI Agents
v3/internal/runtime/desktop/@wailsio/runtime/src/window.ts lines 546-555: the
code currently only applies devicePixelRatio scaling for Windows; update it so
DPI scaling is applied for all platforms (Linux/macOS included) by computing
const scale = window.devicePixelRatio || 1 and then cssX = x / scale and cssY =
y / scale before calling document.elementFromPoint(cssX, cssY); you can simplify
by removing the platform conditional and always using the scaled coordinates
(falling back to x/y when scale === 1) to ensure correct hit-testing for
file-drop handlers on non-Windows platforms.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this guarded to Windows for a reason?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@leaanthony, yes, the Windows-specific guard is based on the author's testing. According to the PR description and linked issue #4631:

  • The issue was reproduced on Windows at non-100% display scaling
  • macOS did not exhibit the issue when tested by the author
  • Linux was not tested

However, CodeRabbit's earlier analysis found TODOs in the Go backend code that suggest this might not be the complete picture:

  • v3/internal/runtime/desktop/linux/webview_window_linux.go line 563: // TODO: proper DPI scaling
  • v3/internal/runtime/desktop/darwin/webview_window_darwin.go line 1152: // TODO: proper DPI scaling

These TODOs indicate that Linux and macOS native layers may also be passing physical (device) pixels rather than CSS pixels, similar to Windows.

The question is: Why does macOS appear to work without the scaling fix?

This could be due to:

  1. macOS might be passing CSS pixels after all (despite the TODO comment)
  2. The test environment had devicePixelRatio = 1.0 on macOS
  3. Some other difference in how macOS WebView handles coordinates

Recommendation: It would be worth verifying the actual coordinate values being passed on macOS and Linux at various display scaling settings. If they're also physical pixels, the fix should be applied universally (removing the platform check) rather than being Windows-specific, which would also address the TODOs in the backend.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@CodeRabbit - If window.devicePixelRatio is supported in wkwebkit and webkit2gtk, wouldn't it just work anyway?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧩 Analysis chain

🌐 Web query:

Does WKWebKit on macOS support window.devicePixelRatio?

💡 Result:

Yes — WKWebView/WKWebKit on macOS exposes window.devicePixelRatio. Safari/WebKit implement the Window.devicePixelRatio API, and WKWebView uses the system WebKit engine (so support depends on the installed macOS/Safari WebKit version). [1][2]

Sources:

  1. MDN — Window.devicePixelRatio. [devicePixelRatio spec/description]. [1]
  2. Construct tutorial — WKWebView on macOS is provided by the system WebKit (version matches the OS/Safari release). [2]

🌐 Web query:

Does WebKit2GTK on Linux support window.devicePixelRatio?

💡 Result:

Short answer: Yes — WebKit2GTK (WebKitGTK) provides window.devicePixelRatio in Linux web views. It is used for HiDPI and GNOME scaling, though historically there have been issues and ongoing bug discussions about exactly how GNOME text-scaling / DPI values are mapped into devicePixelRatio. [1][2]

Sources:

  • MDN docs for window.devicePixelRatio. [1]
  • WebKitGTK bug/maillist discussion about GNOME text-scaling-factor and devicePixelRatio behavior. [2]

@leaanthony, you're absolutely right! Both WKWebKit (macOS) and WebKit2GTK (Linux) support window.devicePixelRatio [1, 2].

Given that:

  1. All platforms support devicePixelRatio — confirmed by the web search results above
  2. The Go backend has TODOs for "proper DPI scaling" on both macOS (webview_window_darwin.go:1152) and Linux (webview_window_linux.go:563), suggesting they might also pass physical pixels
  3. The fix is safe/harmless — when devicePixelRatio === 1 (no scaling), the math becomes x/1 = x, so coordinates remain unchanged

Recommendation: Remove the Windows-specific guard and apply the coordinate scaling universally:

HandlePlatformFileDrop(filenames: string[], x: number, y: number): void {
    const scale = window.devicePixelRatio || 1;
    const cssX = x / scale;
    const cssY = y / scale;
    const element = document.elementFromPoint(cssX, cssY);
    
    // ... rest of the code
}

This simplifies the code, future-proofs it for HiDPI/Retina displays on all platforms, and addresses the backend TODOs. The fact that macOS "didn't exhibit the issue" during testing might simply mean it was tested at 1.0 scaling or the issue manifests differently—but applying the fix universally ensures correctness regardless.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

Copy link
Member

@leaanthony leaanthony Oct 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In which case do you think it would be better that this fix live in Go like the other platforms rather than have a JS fix for Windows only?

Copy link
Author

@yulesxoxo yulesxoxo Oct 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the late reply, github did not seem to send me a notification for your message.

The logic could be moved to webview_window_windows.go specifically func (w *windowsWebviewWindow) convertWindowToWebviewCoordinates

The caveat being that the console.log in HandlePlatformFileDrop(...) e.g.
console.log(`Wails Runtime: Drop on element (or no element) at ${x},${y} which is not a designated dropzone. Ignoring. Element:`, element);
Become practically useless as they now report CSS coordinates instead of physical ones. I imagine this will lead to more frequent bug reports of people saying the coordinates in console.log do not match with what they expect.

So the question is really what behavior do you want?

  1. Go side converts Physical coordinates to CSS coordinates (scaling applied), Frontend uses and logs CSS coordinates
  2. Go side converts Physical coordinates to CSS coordinates (scaling applied), Frontend uses CSS coordinates but converts back to Physical coordinates and logs Physical coordinates for DX
  3. Go side sends Physical coordinates, Frontend needs to know when to convert to CSS coordinates and when to use physical or CSS coordinates.
  4. Go side passes both physical and CSS coordinates, Frontend needs to know when to use which.

If you are asking for my personal opinion:
Firstly I would propose updating the JS doc to clearly indicate whether the x and y coordinates are css or physical or even change the property names.
physicalX, cssX, ... or whatever terminology you prefer.
Secondly if I had to choose between passing only physical or css coordinates I would choose physical ones because looking at it from the PoV as a developer building a desktop app to me it makes a lot more sense to work with physical coordinates and only convert when required. CSS coordinates require mentally mapping to the element’s actual visual position on the screen, whereas physical coordinates match exactly where I see the drop happening.
But if the goal is simply to have the logic in go the clear choice would be Option 4 always pass physical and css coordinates from go to the frontend.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your great thoughts! I'm not too worried about logging (I'm ok with removing it!) but the question is how will the developer use the information returned. If we don't know then perhaps it's better than both are returned? It still feels to me that we shouldn't have windows specific logic in the frontend but translate that when it's received by Go. I think being consistent will aid us in the long run. Thoughts?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@leaanthony moved conversion logic to v3/pkg/application/webview_window_windows.go
Tested on the repro repo and it works.

Also updated the JSDoc comment in v3/internal/runtime/desktop/@wailsio/runtime/src/window.ts for clarity.

I think once v3 is in beta there will be more people voicing their opinions whether they want physical/logical/both pixels in logs and I can revisit this then if needed.

@yulesxoxo yulesxoxo changed the title fix: DnD dropzone detection failing at non-100% display scaling [v3] fix: DnD dropzone detection failing at non-100% display scaling Oct 6, 2025
@yulesxoxo yulesxoxo changed the title [v3] fix: DnD dropzone detection failing at non-100% display scaling [v3] fix: (Windows OS) DnD dropzone detection failing at non-100% display scaling Oct 6, 2025
@yulesxoxo yulesxoxo force-pushed the fix-dropzone-scaling-calculation branch from 9202d30 to 4c68e20 Compare November 4, 2025 00:52
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9202d30 and 5f835c0.

📒 Files selected for processing (3)
  • v3/UNRELEASED_CHANGELOG.md (1 hunks)
  • v3/internal/runtime/desktop/@wailsio/runtime/src/window.ts (1 hunks)
  • v3/pkg/application/webview_window_windows.go (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • v3/UNRELEASED_CHANGELOG.md
  • v3/internal/runtime/desktop/@wailsio/runtime/src/window.ts
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-10-17T23:16:11.570Z
Learnt from: Sammy-T
Repo: wailsapp/wails PR: 4570
File: v2/internal/frontend/desktop/linux/window_webkit6.go:97-108
Timestamp: 2025-10-17T23:16:11.570Z
Learning: For webkit_6/GTK4 builds in v2/internal/frontend/desktop/linux/window_webkit6.go, GTK widget creation should not be wrapped in invokeOnMainThread. The activation mechanism (activateWg + onActivate export) already handles thread safety, and additional wrapping would cause issues.

Applied to files:

  • v3/pkg/application/webview_window_windows.go
📚 Learning: 2024-09-20T23:34:29.841Z
Learnt from: nixpare
Repo: wailsapp/wails PR: 3763
File: v3/examples/keybindings/main.go:16-17
Timestamp: 2024-09-20T23:34:29.841Z
Learning: In the codebase, `application.Options.KeyBindings` uses the `application.Window` type, whereas `application.WebviewWindowOptions.KeyBindings` uses `*application.WebviewWindow`. This is intentional and acceptable.

Applied to files:

  • v3/pkg/application/webview_window_windows.go
🧬 Code graph analysis (1)
v3/pkg/application/webview_window_windows.go (1)
v3/pkg/w32/user32.go (1)
  • GetDpiForWindow (383-386)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: Run Go Tests v3 (ubuntu-latest, 1.24)
  • GitHub Check: Run Go Tests v3 (macos-latest, 1.24)
  • GitHub Check: Run Go Tests v3 (windows-latest, 1.24)
🔇 Additional comments (1)
v3/pkg/application/webview_window_windows.go (1)

8-8: LGTM – math import correctly added.

The math package import is necessary for math.Round used in the DPI scaling calculation.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Nov 4, 2025

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Documentation Improvements or additions to documentation runtime v3-alpha Windows

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants